native: add deparse - #5

@ashbyhq/libpg-query-native mirrors the WASM API — parse, parsePlPgSQL,
fingerprint, normalize, scan — minus the one thing consumers reach outside the
package for. The consumer contract test spells it out: the AST we return gets
handed to pgsql-deparser, a hand-written TypeScript reimplementation of
Postgres' deparseRawStmt that has to track a C file changing every major
release. Being on the 17 line while we parse with 18 is a standing hazard, and
the test carries a PG18_ONLY list of constructs that silently do not survive it.

libpg_query has shipped pg_query_deparse_protobuf() since 2.x, and the pinned
18.0.0 also has pg_query_deparse_protobuf_opts() and
pg_query_deparse_comments_for_query(). So the deparser was already compiled
into the addon — nothing exposed it.

  const { parseSync, deparseSync } = require('@ashbyhq/libpg-query-native');
  deparseSync(parseSync('select a,b   from   t'));  // SELECT a, b FROM t

The obstacle was never the C side. pg_query_deparse_protobuf takes a
protobuf-encoded tree while parse() returns JSON, and pg_query.proto maps
between the two with json_name annotations — 1,683 of them, which is why
SelectStmt and targetList in the JSON correspond to select_stmt and target_list
in the schema. protobufjs ignores json_name, so a parse tree cannot be
re-encoded with it at all. @bufbuild/protobuf honours it.

The generated schema is committed to src/gen/, so npm ci and the platform
builds need no protobuf toolchain. scripts/generate-proto.mjs regenerates it
and refuses to run unless protos/18/pg_query.proto matches
x-upstream.libpgQueryTag — a tree encoded against a mismatched schema deparses
into wrong SQL rather than failing loudly, so that guard is the point.

Also exposed: prettyPrint/indentSize/maxLineLength/trailingNewline/
commasStartOfLine, and extractComments() to carry comments across a round trip
(parse trees don't hold them). Everything but comments is a pretty-print option
upstream and only applies alongside prettyPrint, which the tests pin.

Encoding is strict — a misspelled field or bogus enum value throws rather than
being dropped and deparsed into quietly wrong SQL. Trees the deparser rejects
come back as SqlError with the failing C function and line, same shape as the
existing parse errors.

One real bug found by testing, unique to a 64-bit build: FETCH_ALL is LONG_MAX,
and JSON.parse rounds that to 2^63 — one past the int64 ceiling protobuf
accepts — so `FETCH ALL`, `MOVE ALL` and `FETCH BACKWARD ALL` all failed to
encode. src/proto.ts repairs values that lost precision, returning the input
untouched when there's nothing to fix. (This does not arise under WASM, where
long is 32 bits.)

tsconfig moves to moduleResolution node16 because @bufbuild/protobuf is
exports-only with no typesVersions fallback; emit stays CommonJS.

99 tests pass (53 new). The consumer contract test now round-trips through
native deparse as well as pgsql-deparser, and asserts the PG18 constructs that
pgsql-deparser drops survive ours — verified against the packed tarball, which
also proves the bundled schema ships.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Review follow-ups. Two of these are bugs the original commit shipped.

CodeRabbit flagged DeparseOptions.comments as unbounded, and it reproduces:
a JS array reports `length` up to 2^32-1 no matter how many elements it holds,
and that length drove three reserve() calls and the read loop. A sparse array
with length 2^32-1 took RSS to 17.8 GB and was still climbing after 32 minutes
with the thread wedged — in the package whose whole premise is bounded RSS.
Rejected now at 1e6 with a RangeError: 2 ms, 69 MB, no allocation.

Separately, protobuf-es defaults recursionLimit to 100. Nesting grows about one
level per set operation, so deparse failed on any chain past ~92 UNIONs — a 22x
artificial reduction below what actually works, and well inside what generated
SQL produces. It also meant the 1500-UNION query in benchmark/memory.mjs could
not be deparsed at all.

The limit is load-bearing rather than a quota, which is why it is raised to 2000
and not removed. Measured on darwin-arm64 / Node 24: the JS stack gives out
around 2050 levels with a bare RangeError, and if --stack-size is raised so JS
survives, deparseRawStmt on the C side has no depth guard of its own and
segfaults around 8000. 2000 sits under both, so the failure is a message that
says what happened instead of a crash. Both failure modes — protobuf-es's plain
Error and the stack's RangeError — now surface as one RangeError naming the
limit, with the original attached as `cause`.

Memory review of the rest, measured on a 26 MB parse tree:

- The int64 repair walk ran Object.entries() per object node, allocating a pair
  array on every node of every deparse for a pass that almost never changes
  anything. for..in instead: 159 ms -> 8 ms, 6% of encode down to ~0%.
- DeparseSync copied the deparsed SQL into a std::string and then into a V8
  string. Added a const char* overload of ReturnResult so libpg_query's buffer
  goes straight to V8 — one full copy of the output saved.

What is not a defect, having checked: RSS climbs ~600 MB after the first deparse
and ratchets under the system allocator, but the JS heap stays flat (heapUsed
71 MB across four cycles) so it is native, and libpg_query's deparse path frees
correctly — MemoryContextDelete plus free(result.query), both of which we call.
It is allocator retention, the same characteristic the README already documents
for parse, and jemalloc stabilizes it (876/898/978/980 MB system vs
510/568/556/562 MB jemalloc). Documented rather than chased.

The remaining cost is inside protobuf-es: toBinary is 2255 ms of the 2581 ms
encode and fromJson builds a ~193 MB transient message graph. Avoiding that
needs a hand-written JSON-to-protobuf encoder, which is not worth the risk here.

106 tests pass (6 new, covering 50/100/500/1500-way UNION chains, the depth
failure message, and the sparse comment array). Consumer contract re-verified
against the packed tarball.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Encoding was ~10x slower than it needed to be. @bufbuild/protobuf is
reflection-driven and, per nested message, allocates a message object on the
way in and two chunk arrays on the way out. pg_query trees are pathologically
nested — every value is wrapped in a Node — so a 26 MB parse tree is ~1.44M
messages, and the encode spent 2571 ms building ~193 MB of intermediates to
produce 6.8 MB of wire bytes.

protobufjs JIT-compiles a per-type encoder and writes from plain objects. Same
tree: 241 ms. End to end a deparse goes 2948 ms -> 465 ms, JS heap high-water
201 MB -> 74 MB, RSS ~980 MB -> ~810 MB.

The reason this wasn't the obvious choice originally is that protobufjs is
famous for ignoring json_name, which is exactly what the parse tree is keyed by
— it's why the earlier attempt upstream vendored a protobufjs fork. But that is
only true of its *converters*. Its parser retains the annotation and exposes it
as Field.jsonName, which also supplies the proto3 lowerCamelCase default for the
30 of 1,713 fields that declare no json_name (Integer.ival, String.sval,
ParseResult.stmts). So the bridge is a key rename driven off the descriptor.

Two things that rename has to carry, both commented at length in src/proto.ts
because neither is obvious from the surrounding code:

- Strictness. protobufjs is permissive by design: fromObject() drops unknown
  keys and turns an unrecognised enum name into 0, both silently. For a
  deparser that is the worst available failure mode — valid-looking SQL that
  doesn't match the tree the caller passed, with nothing raised. @bufbuild
  rejected both by default and this API documents that behaviour, so the remap
  enforces it directly. Not via protobufjs's verify(), which would be a second
  full traversal; the remap already visits every key holding the field
  descriptor, so the checks are free there.

- The 64-bit repair. Previously a separate pass over the tree; now inline in
  the remap, which is visiting every scalar anyway. Same for the depth bound,
  which is now counted during the walk rather than by a pre-pass.

protobufjs enforces its own recursion cap, and it defaults to 100 — the same
too-low value that capped deparse at ~92 set operations before. It lives on a
module global rather than per-Root, so it is raised alongside RECURSION_LIMIT.

Wire output is unchanged, and that is now pinned rather than asserted:
test/fixtures/encoded-parse-trees.json holds golden encodings captured from the
@bufbuild implementation across 37 statements — enums, oneofs, the int64
FETCH ALL, floats, set operations, DDL, PG18-only constructs — and
test/proto.test.js requires this encoder to reproduce them byte for byte.

Trade-off worth naming: the package tarball shrinks 129 kB -> 51 kB, but
protobufjs (3.9 MB) is bigger than @bufbuild/protobuf (1.9 MB), so installs grow
about 2 MB.

152 tests pass (46 new). Consumer contract re-verified against the packed
tarball.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

encodeEnum validated enum names but let any number through, so the strictness
the surrounding code documents applied to half the input. CodeRabbit caught the
inconsistency; the impact is worse than inconsistency.

An unmapped value is not a harmless passthrough. libpg_query's deparser takes
its default branch and drops the construct without raising anything:

  tree.stmts[0].stmt.SelectStmt.op = 999;
  deparseSync(tree);   // "SELECT"   — was `SELECT a UNION SELECT b`

The set operation and both arms are gone. That is precisely the silent-wrong-SQL
failure this encoder rejects unknown keys and enum names to avoid, reachable
through the one path that skipped the check. Non-integers and NaN went through
the same way.

Validated against valuesById, which is keyed by wire number, so one lookup
covers unmapped values, non-integers and NaN together. The review suggested
Object.values(...).includes(...); that allocates an array and scans every member
on a path that runs once per enum in the tree, and valuesById gives the same
answer in O(1) with no allocation.

Six tests: each of 999, -5, 1.5, NaN and Infinity rejected, plus a non-numeric,
non-string value. The existing test that a valid wire number still encodes
identically to its name is unchanged.

160 tests pass.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The comment round-trip example destructured only extractCommentsSync but then
called deparseSync and parseSync, so copying that block verbatim threw a
ReferenceError. Every other example in the file imports what it calls.

Verified by extracting the block from README.md and executing it, rather than
by reading it:

  "-- keep me\n SELECT a FROM t"

Caught by CodeRabbit.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

protobufjs builds Enum#values as Object.create(valuesById), so the number->name
reverse mapping is inherited by the name->number map. A numeric string therefore
resolves instead of being absent:

  values["SETOP_UNION"] === 2                      (own)
  values["2"]           === "SETOP_UNION"          (inherited)

encodeEnum treated any non-undefined result as a hit, so it returned a string
from a function declared to return a number, and protobufjs coerced it:

  op: "0"  ->  "SELECT"                    silently dropped the UNION and both arms
  op: "1"  ->  "SELECT"                    same
  op: "2"  ->  "SELECT a UNION SELECT b"   correct, but only by accident

Only strings matching a real wire number leaked through; "999" already threw,
because valuesById has no such key either. Guarded with hasOwnProperty on
enumType.values, which also makes the declared return type honest again.

Audited the encoder's other lookups: valuesById is Object.create(null), so the
numeric path was never exposed, the json_name table is a Map, and the tree read
is already behind the own-property guard added in 6a95729. This was the only
one.

Four tests: "0", "1", "2" rejected, plus a string naming a prototype member. The
existing cases — SETOP_UNION by name, 2 as a wire number, and the 37 golden
wire-format fixtures — are unchanged.

Third prototype-inheritance bug in this PR, and CodeRabbit found two of them.

164 tests pass.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Two review rounds caught examples that don't run when copied: one missing its
imports, and the pretty-print block, which had no require at all, referenced a
`tree` defined in a different code block, and showed output for a different
query than the one that tree came from. Both only surface when someone pastes
the snippet and it throws.

Fixed the pretty-print block to stand alone, and corrected the comment example's
documented output — the comment is re-inserted with a leading space before the
statement, which the README did not show.

Added test/readme.test.js so this stops being something review has to catch. It
extracts every ```js block, asserts a block calling the API also imports it,
executes it, and rewrites each `call();` followed by `// expected` lines into an
assertion so the documented output is checked rather than assumed.

Confirmed it bites rather than passing vacuously, against both failure modes
seen in review:

  wrong documented output  -> block 1 fails on the mismatch
  removed import           -> ReferenceError: deparseSync is not defined

173 tests pass.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Cleanup pass over the deparse work. No behaviour change: the 37 wire-format
fixtures are byte-identical throughout, which is what pins that.

Reuse:

- generate-proto.mjs read the pin from package.json's x-upstream. That field is
  *derived* — sync-upstream-metadata.mjs writes it — so a Makefile bump that
  had not been synced would leave the drift guard comparing against the stale
  tag and passing, which is the exact failure the guard exists to catch. It now
  calls currentPin() from upstream.mjs, whose header already declares the
  Makefile the single source of truth, and builds the fetch URL from the pinned
  repo rather than hardcoding pganalyze.
- ExtractCommentsSync hand-built the {error, result} envelope that ReturnResult
  builds. Added a Napi::Value overload, which also retires the identical
  hand-rolled tail in the pre-existing ScanSync.

Simplification:

- pg_query_deparse_protobuf() is exactly pg_query_deparse_protobuf_opts() with
  a zeroed opts struct (libpg_query/src/pg_query_deparse.c:16-22), and opts is
  already value-initialised here, so the has_opts flag and its two-branch call
  collapse to one entry point.
- remapMessage opened with an Array.isArray branch that nothing could reach:
  arrays are handled by the field.repeated ternary before the call. It also
  disagreed with the live path, passing depth unchanged where the loop passes
  depth + 1, so the recursion guard would have stopped counting if it ever were
  reached. Removed.
- Dropped Has() guards whose following expression already implies them (a
  missing key reads back as undefined, which is falsy and not a number) and the
  double Get() in intOpt; hoisted arr.Get(i); folded the three copy-pasted
  comment int extractions into a lambda; recorded the comment pointer in the
  same loop rather than a second pass over comment_storage.
- deparseSync's ternary existed only to avoid passing undefined, which the
  addon already treats as absent.

Tests:

- The round-trip suite and the wire-format fixtures kept two hand-written SQL
  lists that had drifted apart in whitespace. They assert different properties
  over the same statements, so they now share test/fixtures/corpus.js. The
  round-trip corpus grew 22 -> 37 as a result: more coverage from less source.
- Added scripts/generate-fixtures.mjs. The fixtures had no regeneration path at
  all — they were captured from @bufbuild/protobuf by a throwaway script — while
  the test told maintainers to "only regenerate against a known-good
  implementation". The script requires --confirm, documents that regenerating
  from the encoder under test is self-approving, and records how to re-establish
  the independent cross-check when the pin moves. Verified it reproduces the
  committed fixtures byte for byte.
- smoke.mjs runs on all five platforms in CI and covered every public API except
  the new one, so deparse was only ever exercised on a dev machine. Added three
  cases: a round trip, unknown-field rejection, and comment extraction.
- Deduped the two identical round-trip walks in consumer-contract.mjs section 6.

Skipped: sharing the position-stripping helper with parsing.test.js. The three
copies use different key sets, and unifying them would change what a
pre-existing test considers structurally equal.

188 tests pass, consumer contract verified against the packed tarball.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The simplify pass moved this script onto currentPin(), which reads
LIBPG_QUERY_TAG from the Makefile — the declared single source of truth — but
the header still told the reader the schema must match x-upstream.libpgQueryTag
in package.json. x-upstream is the derived copy, so the comment named the very
source the change was made to stop trusting.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…to do

Third instance of the same defect, one layer below the last one. After the
palloc patch, a profile of the deparse path put 84% of samples inside
protobuf_c_message_unpack's own body with the allocator down in the noise. The
cause is the loop that closes out an unpack: it walks every field of the
message descriptor to allocate arrays for repeated fields and to verify
required fields.

pg_query's Node is the pathological input for that loop. It has 271 fields, it
is proto3 so nothing is required, and it is a oneof of singular message fields
so nothing is repeated -- and every value in a parse tree is wrapped in a Node.
All 271 iterations are no-ops, once per node in the tree.

patches/protobufc_skip_noop_field_loop.patch guards the loop with a memoized
per-descriptor "has any repeated or required field?" check. List, SelectStmt
and everything else with genuinely repeated fields still run it; Node skips it.
The memo packs descriptor pointer and answer into a single word, which cannot
tear, so concurrent unpacks either see a complete entry or recompute -- no lock
and no torn state. The loop body is left un-reindented to keep the change two
lines and rebase cleanly.

This one patches vendored third-party code rather than libpg_query's own, so
it was verified against a stock build directly: unpacking and re-packing
reproduces the input byte-for-byte on all 45 payloads (the 42-statement corpus
plus three shapes), with checksums compared between stock and patched binaries.
Plus the usual gates on a from-scratch clone+patch+build: 205 tests, the
47-statement differential, 19 adversarial cases.

Measured (darwin-arm64), on top of the palloc patch:

  unpack                3.0-3.2x faster
  deparse, select        4.9 -> 3.4 us
  deparse, cte           9.2 -> 6.2 us
  deparse, wide-100     69.6 -> 41.1 us

Against pgsql-deparser on a parse->edit->deparse flow, native is now at parity
on small statements (1.02x) and faster on wide ones (0.83x). Encode is now the
largest single component of a wide deparse at ~42%.

Memory is unchanged in shape: flat across repeated cycles under both
allocators, ~70 MB steady state at 126k ops/sec on ordinary statements.

Upstream home is protobuf-c, not libpg_query -- noted for whoever takes it
there. libpg_query's use-upb-for-protobufs branch would moot both patches.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

deparseStringLiteral wraps any value containing a backslash in E'' and
doubles the backslashes. Its comment explains why: it is copied from
postgres_fdw/deparse.c, which ships SQL to a remote server whose
standard_conforming_strings it cannot see, so it picks the spelling that is
safe under either setting. A general-purpose deparser has no remote server,
and Postgres' own parse-tree-to-SQL path, simple_quote_literal() in
ruleutils.c, does the opposite and says "we never use E''".

The divergence is observable: pg_get_constraintdef() on CHECK (c ~ '^\d+$')
returns a plain literal where deparsing the same tree returned E'^\\d+$'.
Ordinary statements failed the textual round-trip these tests are built on,
including SELECT regexp_replace(x, '\s+', ' ').

It also breaks non-Postgres consumers. E'' is a Postgres extension;
ClickHouse lexes the E as an identifier and rejects the query, which is a
production bug we are fixing on the Ashby side. Postgres made this same
change in 2006 so pg_dump output could load into other databases without
the backslash doubling.

Keying the doubling off standard_conforming_strings rather than hardcoding
true also honours PG_QUERY_DISABLE_STANDARD_CONFORMING_STRINGS, which this
library exposes and the deparser previously ignored.

The behaviour is pinned by tests rather than left to the patch applying,
since the spelling is the part consumers depend on. Callers that pre-escape
a value for another dialect at the tree level now get it through untouched
instead of doubled by a second escaping pass.

Upstreaming needs one expectation updated: deparse_tests.c pins CREATE
DOMAIN us_postal_code with E'^\\d{5}$' inputs, copied from the Postgres
docs, which now round-trip as plain literals. We do not run that suite
here, so the patch stays minimal.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>